MGMT-21200: Enable dual-stack#390
MGMT-21200: Enable dual-stack#390openshift-merge-bot[bot] merged 1 commit intorh-ecosystem-edge:mainfrom
Conversation
|
@danmanor: This pull request references MGMT-21200 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughThis change introduces dual-stack (IPv4 and IPv6) support for cluster IP and machine network CIDR configuration throughout the codebase and configuration files. It updates struct fields, CLI parameters, and YAML configuration to accept lists of addresses/CIDRs. The etcd and postprocessing logic is refactored to handle dual-stack renaming and resource updates, and robust checks are added for IP replacements. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant ConfigParser
participant PostProcess
participant EtcdClient
participant FileSystem
CLI->>ConfigParser: Parse config (IPs, CIDRs as lists)
ConfigParser->>PostProcess: Provide parsed IPs/CIDRs
PostProcess->>EtcdClient: Extract original IPs (dual-stack)
alt Single IP
PostProcess->>EtcdClient: Rename resources (IPv4)
PostProcess->>FileSystem: Rename files/dirs (IPv4)
else Dual-stack IPs
PostProcess->>EtcdClient: Rename resources (IPv4 & IPv6)
PostProcess->>FileSystem: Rename files/dirs (IPv4 & IPv6)
end
PostProcess->>EtcdClient: Update machine network CIDRs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Suggested labels
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (7)
🧰 Additional context used🧬 Code Graph Analysis (1)src/ocp_postprocess/ip_rename/etcd_rename.rs (2)
🔇 Additional comments (14)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Hi @danmanor. Thanks for your PR. I'm waiting for a rh-ecosystem-edge member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@danmanor: This pull request references MGMT-21200 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@danmanor: This pull request references MGMT-21200 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@danmanor: This pull request references MGMT-21200 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@danmanor: This pull request references MGMT-21200 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@danmanor: This pull request references MGMT-21200 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@danmanor: This pull request references MGMT-21200 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@danmanor: This pull request references MGMT-21200 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
danmanor
force-pushed
the
enable-dual-stack
branch
from
fd0fe1d to
c4a40a4
Compare
|
@danmanor: This pull request references MGMT-21200 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@danmanor: This pull request references MGMT-21200 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/ocp_postprocess/machine_config_cidr_rename/etcd_rename.rs (1)
34-42: Consider adding CIDR validation.The dual-stack CIDR handling logic is correct and properly handles comma-separated inputs. However, consider adding validation to ensure each CIDR is valid and not empty after trimming.
let new_networks: Vec<_> = machine_config_network .split(',') + .filter(|cidr| !cidr.trim().is_empty()) .map(|cidr| serde_json::json!({"cidr": cidr.trim()})) .collect();src/config.rs (2)
225-238: Consider refactoring duplicated parsing logic.The parsing logic for handling both single strings and arrays is duplicated between the
ipandmachine_network_cidrfields. Consider extracting this into a helper function.fn parse_string_or_array(value: Value, field_name: &str) -> Result<Option<Vec<String>>> { match value { Value::Array(array) => { Some(array.iter() .map(|v| -> Result<String, anyhow::Error> { Ok(v.as_str() .with_context(|| format!("{} array element must be a string", field_name))? .to_string()) }) .collect::<Result<Vec<_>, _>>()?) } Value::String(single) => Some(vec![single]), _ => anyhow::bail!("{} must be a string or array of strings", field_name), } }Also applies to: 273-285
452-452: Consider adding validation for comma-separated inputs.The CLI parsing splits on commas and trims, but doesn't validate that the resulting strings are non-empty or valid IPs/CIDRs.
- ip: cli.ip.map(|s| s.split(',').map(|s| s.trim().to_string()).collect()), + ip: cli.ip.map(|s| s.split(',') + .map(|s| s.trim()) + .filter(|s| !s.is_empty()) + .map(String::from) + .collect()),Also applies to: 459-459
src/ocp_postprocess/ip_rename/etcd_rename.rs (1)
13-16: Consider using the standard library's IP parsing directly.The helper could be simplified by leveraging the IP address parsing more directly.
-fn is_ipv6(ip: &str) -> bool { - ip.parse::<IpAddr>().map(|addr| addr.is_ipv6()).unwrap_or(false) -} +fn is_ipv6(ip: &str) -> bool { + ip.parse::<Ipv6Addr>().is_ok() +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.gitignore(0 hunks)hack/dummy_config.yaml(2 hunks)src/config.rs(4 hunks)src/config/cli.rs(2 hunks)src/etcd_encoding.rs(3 hunks)src/ocp_postprocess.rs(3 hunks)src/ocp_postprocess/ip_rename.rs(2 hunks)src/ocp_postprocess/ip_rename/etcd_rename.rs(9 hunks)src/ocp_postprocess/machine_config_cidr_rename/etcd_rename.rs(2 hunks)
💤 Files with no reviewable changes (1)
- .gitignore
🔇 Additional comments (16)
src/config/cli.rs (1)
100-100: LGTM: Clear dual-stack documentation.The documentation updates clearly explain the expected comma-separated format for dual-stack configurations and properly specify the IPv4-first ordering requirement.
Also applies to: 153-153
hack/dummy_config.yaml (1)
45-47: LGTM: Comprehensive dual-stack configuration example.The dummy configuration properly demonstrates dual-stack networking with consistent IPv4-first ordering across all network components (IPs, cluster networks, machine networks, and service networks).
Also applies to: 74-82, 89-91
src/config.rs (1)
45-45: LGTM: Proper data structure changes for dual-stack support.The field type changes from
Option<String>toOption<Vec<String>>correctly support multiple IPs and CIDRs for dual-stack networking.Also applies to: 53-53
src/ocp_postprocess.rs (3)
155-163: LGTM! Well-structured dual-stack IP handling.The implementation correctly handles both single IP (backward compatible) and dual-stack scenarios with appropriate logging.
205-211: LGTM! Clean handling of multiple machine network CIDRs.The approach of joining CIDRs with commas maintains backward compatibility while supporting dual-stack configurations.
924-936: LGTM! Consistent implementation pattern.The new function follows the established pattern for rename operations with proper error context.
src/ocp_postprocess/ip_rename.rs (4)
18-30: Good separation of concerns with pre-extraction pattern.The approach of extracting original IPs before making any modifications prevents potential race conditions and ensures data integrity.
86-140: Excellent refactoring for code reuse.The helper function effectively consolidates all etcd fixes and enables both single and dual-stack code paths to share the same logic.
142-186: Clean separation of IP extraction logic.The function properly extracts the original IP without side effects and correctly handles IPv6 bracket formatting.
198-220: Well-structured dual-stack processing with proper validation.The function correctly validates the presence of both IPs and provides clear logging for troubleshooting.
src/ocp_postprocess/ip_rename/etcd_rename.rs (6)
26-66: Well-implemented dual-stack IP extraction logic.The function correctly handles node address extraction with proper validation for single-node scenarios and clear error messages.
84-110: Excellent defensive programming with conditional replacement.The function properly validates the presence of the original IP before making changes, preventing unintended modifications.
114-140: Robust URL replacement logic.The function correctly validates URL presence before replacement and handles IPv6 formatting consistently.
486-490: Good handling of optional IPv6 arguments in dual-stack.The function correctly handles the case where IPv6 arguments might not be present in certain configurations, with clear logging for operators.
538-580: Excellent refactoring of etcd member update logic.The function now properly queries etcd state, validates single-node assumptions, and only updates when the original IP is actually present. This is much more robust than blind replacement.
27-27: Confirm deprecated “minions” prefix in etcd key lookup
- File:
src/ocp_postprocess/ip_rename/etcd_rename.rs, line 27
let node_keys = etcd_client.list_keys("minions").await?;- The term
"minions"was renamed to"nodes"in Kubernetes v1.0+.- Please verify whether your OpenShift/Kubernetes version still stores node entries under
"minions"in etcd. If it now uses"nodes", update this call accordingly.
| ip.parse::<IpAddr>().map(|addr| addr.is_ipv6()).unwrap_or(false) | ||
| } | ||
|
|
||
| // Extract both original IPv4 and IPv6 IPs from dual-stack cluster node configuration |
There was a problem hiding this comment.
We should have this information provided to us, we already get the original IP from LCA (which gets it from the seed manifest seed image label), why do we need to extract it from the nodes?
There was a problem hiding this comment.
How do we get it ? do you mean in the cn_san_replace_rules ? otherwise AFAIU the recert config only contains desired state not seed state no ?
There was a problem hiding this comment.
Oh nvm, I thought we get it as an explicit argument, but we just extract it from the cluster. Ignore me
danmanor
force-pushed
the
enable-dual-stack
branch
from
c4a40a4 to
ce0c1d7
Compare
|
@danmanor: This pull request references MGMT-21200 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/ocp_postprocess/ip_rename/etcd_rename.rs (1)
17-21: Address the past review comment about IP extraction.There's a valid concern from a previous review questioning why we need to extract original IPs from nodes when this information should be provided by LCA. This adds complexity and potential points of failure.
Consider whether this extraction logic is necessary or if the original IP information can be obtained from LCA as suggested in the previous review.
🧹 Nitpick comments (4)
src/ocp_postprocess/ip_rename/etcd_rename.rs (3)
12-15: Consider more robust IPv6 detection.The current implementation will return
falsefor invalid IP strings, which might mask parsing errors. Consider being more explicit about error handling.-fn is_ipv6(ip: &str) -> bool { - ip.parse::<IpAddr>().map(|addr| addr.is_ipv6()).unwrap_or(false) -} +fn is_ipv6(ip: &str) -> Result<bool> { + let addr = ip.parse::<IpAddr>().context("Invalid IP address format")?; + Ok(addr.is_ipv6()) +}Alternatively, if you want to keep the current signature for simplicity, add a comment explaining the behavior for invalid IPs.
23-78: The IP extraction logic is well-implemented but has room for improvement.The function correctly handles both single-stack and dual-stack scenarios with proper error handling and logging.
However, consider these improvements:
- The function assumes IPv4 comes before IPv6, but this might not always be guaranteed in node addresses:
let mut result = Vec::new(); -if let Some(ipv4) = original_ipv4 { - result.push(ipv4); -} - -if let Some(ipv6) = original_ipv6 { - result.push(ipv6); -} +// Ensure consistent ordering: IPv4 first, then IPv6 +if let Some(ipv4) = original_ipv4 { + result.push(ipv4); +} +if let Some(ipv6) = original_ipv6 { + result.push(ipv6); +}
- Consider adding validation for the extracted IP addresses to ensure they're valid.
550-591: Excellent rewrite using etcd client API with robust IPv6 support.The complete rewrite to use the etcd client API instead of direct manipulation is a significant improvement. The function properly:
- Validates there's exactly one member (correct for SNO)
- Uses proper IPv6 address parsing instead of string manipulation
- Only updates when the original IP is found in peer URLs
- Provides detailed logging
Minor suggestion for consistency:
- let contains_original = current_member.peer_urls().iter().any(|url| url == &expected_original_url); + let contains_original = current_member.peer_urls().iter().any(|url| *url == expected_original_url);This avoids the unnecessary reference in the comparison.
src/ocp_postprocess/machine_config_cidr_rename/etcd_rename.rs (1)
34-42: Good implementation of dual-stack CIDR support.The changes correctly enable dual-stack support by splitting comma-separated CIDRs and replacing the machine network array. The approach is clean and handles both single-stack and dual-stack scenarios.
Consider adding validation for the CIDR format:
let new_networks: Vec<_> = machine_config_network .split(',') - .map(|cidr| serde_json::json!({"cidr": cidr.trim()})) + .map(|cidr| { + let trimmed = cidr.trim(); + // Basic CIDR validation could be added here if needed + serde_json::json!({"cidr": trimmed}) + }) .collect();This would provide a place to add CIDR validation if needed in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.gitignore(0 hunks)hack/dummy_config.yaml(2 hunks)src/config.rs(4 hunks)src/config/cli.rs(2 hunks)src/etcd_encoding.rs(3 hunks)src/ocp_postprocess.rs(3 hunks)src/ocp_postprocess/ip_rename.rs(3 hunks)src/ocp_postprocess/ip_rename/etcd_rename.rs(9 hunks)src/ocp_postprocess/machine_config_cidr_rename/etcd_rename.rs(2 hunks)
💤 Files with no reviewable changes (1)
- .gitignore
✅ Files skipped from review due to trivial changes (1)
- src/config/cli.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- src/config.rs
- src/etcd_encoding.rs
- hack/dummy_config.yaml
- src/ocp_postprocess/ip_rename.rs
- src/ocp_postprocess.rs
🔇 Additional comments (6)
src/ocp_postprocess/ip_rename/etcd_rename.rs (6)
80-124: Excellent improvement with conditional replacement logic.The addition of conditional replacement logic significantly improves safety by only modifying configurations when the original IP is actually present. The IPv6 bracket formatting is handled correctly.
This prevents potential configuration corruption that could occur with unconditional replacements when the original IP isn't found in the expected location.
126-152: Well-implemented conditional replacement with proper IPv6 handling.The function correctly handles IPv6 address formatting with brackets and only performs replacements when the original IP is found in the current configuration.
The logging provides good visibility into when replacements are skipped, which will be helpful for debugging.
204-255: Consistent implementation of conditional replacement logic.The function properly implements the same conditional replacement pattern as other functions, only modifying configurations when the original IP is found.
The logging provides good visibility into when replacements are skipped.
459-518: Good handling of dual-stack scenarios with informative logging.The function correctly handles cases where the etcd server argument might not be found (normal for IPv6 in dual-stack scenarios) with appropriate logging. The IPv6 formatting with brackets and quotes is handled properly.
The early return when the argument is not found prevents unnecessary processing and provides clear feedback about what's happening.
427-440: Clean integration with updated storage config logic.The function correctly delegates to the updated
fix_storage_configfunction that now includes conditional replacement logic.
520-548: Proper conditional replacement for network cluster annotations.The function correctly implements conditional replacement logic, only updating the annotation when the original IP is found. The logging provides good visibility into different scenarios (annotation not found vs. value mismatch).
|
@omertuc Can you please add /ok-to-test ? |
|
/ok-to-test |
Background / Context The lifecycle-agent is a core component responsible for managing Image-Based Upgrades (IBU) and Image-Based Install (IBI) operations in OpenShift Single Node OpenShift (SNO) clusters. It handles critical cluster lifecycle operations including: - **Network Configuration Management**: Configuring node IPs, machine networks, cluster networks, and service networks during cluster transitions - **Recertification (Recert)**: Re-signing certificates with updated cluster information (IPs, hostnames, etc.) when transforming seed images into target clusters - **Post-Pivot Operations**: Managing network setup, DNS configuration, and kubelet configuration after cluster pivot operations - **Seed Cluster Information**: Capturing and transforming network details from seed clusters for target cluster deployment Currently, the lifecycle-agent's network handling is designed around single-stack networking, where clusters operate on either IPv4 OR IPv6, but not both simultaneously. All network-related data structures use single string fields (`NodeIP`, `MachineNetwork`) and the logic assumes a single IP address per network interface. Key components involved: - `api/seedreconfig`: Defines the `SeedReconfiguration` API for cluster transformation parameters - `utils/client_helper.go`: Extracts cluster network information from Kubernetes API - `lca-cli/postpivot`: Handles post-upgrade network configuration and kubelet setup - `internal/recert`: Manages certificate re-signing with updated network information - `lca-cli/seedclusterinfo`: Captures seed cluster network details for replication Issue / Requirement / Reason for change **[MGMT-21201](https://issues.redhat.com//browse/MGMT-21201)**: The lifecycle-agent needs to support dual-stack networking configurations where OpenShift clusters operate with both IPv4 and IPv6 addresses simultaneously on the same interfaces. Current Limitations: 1. **Single IP Assumption**: All network fields (`NodeIP`, `MachineNetwork`) are single strings, preventing multiple IP support 2. **Limited Network Discovery**: Cluster info extraction only captures the first internal IP address 3. **Inadequate Recert Logic**: Certificate re-signing only handles single IP changes 4. **Kubelet Configuration**: Node IP hint generation assumes single network per stack 5. **Missing Test Coverage**: No validation for dual-stack scenarios Requirements: - Support IPv4 + IPv6 dual-stack clusters in IBU/IBI operations - Maintain 100% backward compatibility with existing single-stack configurations - Handle multiple machine networks per IP family - Update recert logic to process multiple IP addresses in certificate SANs - Ensure proper kubelet configuration for dual-stack node IPs Changes Made API Extensions - **Added `NodeIPs []string`** to `SeedReconfiguration` and `SeedClusterInfo` for multiple node IPs - **Added `MachineNetworks []string`** to support multiple machine network CIDRs - **Preserved legacy fields** (`NodeIP`, `MachineNetwork`) for backward compatibility with precedence rules Network Configuration Updates - **Enhanced `GetClusterInfo()`** to discover all internal node IPs via `getNodeInternalIPs()` - **Added `getMachineNetworks()`** to extract all machine networks from install config - **Implemented backward compatibility** by populating legacy fields with first array element Post-Pivot Improvements - **Updated `setNodeIpHint()`** to generate space-separated IP hints: `KUBELET_NODEIP_HINT=<ip1> <ip2>` - **Refactored `setNodeIPIfNotProvided()`** to parse kubelet config from `/etc/systemd/system/kubelet.service.d/20-nodenet.conf` - **Added `parseKubeletNodeIPs()`** function to extract both `KUBELET_NODE_IP` and `KUBELET_NODE_IPS` environment variables - **Enhanced file validation** to check for both existence AND valid content before triggering `nodeip-configuration` service Recertification Logic - **Implemented `slices.Equal()`** comparison for clean IP change detection - **Updated config IP format** to comma-separated list: `config.IP = "ip1,ip2"` for multiple addresses - **Enhanced certificate SAN rules** to include all old and new IP addresses in replacement rules Test Scenarios Covered: - ✅ Single-stack IPv4 and IPv6 configurations - ✅ Dual-stack (IPv4 + IPv6) with both primary orders - ✅ Multiple networks per IP family - ✅ Legacy → new field migration scenarios - ✅ Error handling and edge cases - ✅ Backward compatibility validation Backward Compatibility **100% backward compatibility maintained**: - Existing single-stack clusters continue to work without modification - Legacy `NodeIP` and `MachineNetwork` fields preserved and populated - New fields (`NodeIPs`, `MachineNetworks`) take precedence when specified - All existing APIs and behavior unchanged for single-stack scenarios Testing - **All existing tests pass** - no regressions introduced - **59 new test cases** covering all dual-stack scenarios - **Production-ready validation** for IPv4, IPv6, and dual-stack configurations - **Edge case coverage** including empty configs, invalid data, and service interactions Other related PRs recert - rh-ecosystem-edge/recert#390
omertuc
left a comment
omertuc
left a comment
There was a problem hiding this comment.
Looks great, a few more small things I noticed but otherwise lgtm
danmanor
force-pushed
the
enable-dual-stack
branch
from
e9c011b to
15438a9
Compare
|
@danmanor: This pull request references MGMT-21200 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
danmanor
force-pushed
the
enable-dual-stack
branch
2 times, most recently
from
7d0cbef to
368516c
Compare
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danmanor, omertuc The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@danmanor: This pull request references MGMT-21200 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/ok-to-test |
danmanor
force-pushed
the
enable-dual-stack
branch
from
368516c to
3ee6a5a
Compare
|
/lgtm |
|
/override ci/prow/e2e-aws-ovn-single-node-recert-parallel |
|
@danmanor: danmanor unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file, and the following github teams:. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/override ci/prow/e2e-aws-ovn-single-node-recert-parallel ci/prow/e2e-aws-ovn-single-node-recert-serial |
|
@mresvanis: Overrode contexts on behalf of mresvanis: ci/prow/e2e-aws-ovn-single-node-recert-parallel, ci/prow/e2e-aws-ovn-single-node-recert-serial DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
openshift-merge-bot
bot
merged commit f409769
into
rh-ecosystem-edge:main
4 participants
Dual Stack Networking Support for Recert
This PR implements comprehensive dual stack networking support in the recert tool, enabling transformation from one dual stack configuration to another with conditional replacement logic.
Overview
The recert tool now supports dual stack networking configurations, allowing clusters to operate with both IPv4 and IPv6 addresses simultaneously. This includes intelligent conditional replacement that only modifies resources when the original IP addresses are actually present.
Key Components Supporting Dual Stack
Changes Made
1. Configuration Structure Changes
ClusterCustomizationsstruct: Changedipfield fromOption<String>toOption<Vec<String>>machine_network_cidrfield: Changed fromOption<String>toOption<Vec<String>>--ip 192.168.1.100,2001:db8::100and--machine-network-cidr 192.168.1.0/24,2001:db8::/642. Network Processing Updates
3. Conditional Replacement Logic (Major Enhancement)
Problem Solved
Previously, etcd resource modification functions would unconditionally replace IP addresses, potentially corrupting configurations when original IPs were not present.
Solution Implemented
Functions Updated with Conditional Logic
fix_etcd_endpoints: Only replaces endpoint IPs when original IP matches current valuefix_openshift_apiserver_configmap: Checks storage config URLs before modificationfix_openshiftapiservers_cluster: Validates storage config contains original IPfix_networks_cluster: Verifies annotation values before updatingfix_etcd_member: Fetches actual member configuration and validates peer URLsfix_storage_config: Helper function now requires original IP verificationTechnical Implementation
Processing Logic
IP Address Processing
Network Configuration Processing
Etcd Resource Management
Function Reorganization
fix_etcd_resources_for_ip_pairoriginal_ipandnew_ipparametersBackward Compatibility
Benefits
Testing
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style